Skip to content

feat: late-bound handlers — createServer accepts a handler factory - #55

Open
gmaclennan wants to merge 4 commits into
mainfrom
feat/late-bound-handler
Open

feat: late-bound handlers — createServer accepts a handler factory#55
gmaclennan wants to merge 4 commits into
mainfrom
feat/late-bound-handler

Conversation

@gmaclennan

@gmaclennan gmaclennan commented Aug 20, 2026

Copy link
Copy Markdown
Member

Adds a way for a server's handler to be supplied lazily and replaced over the server's lifetime, while clients notice nothing. The mental model: the channel and its client-visible subscriptions are durable; the handler is a replaceable plug-in behind them.

API

createServer(handlerOrFactory, port, opts) now also accepts a function () => object | Promise<object> — a handler factory. Passing an object binds statically, exactly as today. The overload is unambiguous and backward compatible: createServer previously rejected functions, so the new form claims only previously-invalid input, and no property name on handler objects becomes reserved.

A factory-backed server starts unbound. The first method call or subscription invokes the factory (single-flight — concurrent arrivals share one invocation) and binds the result. The server keeps its subscription registry independently of any particular handler, and on every bind re-attaches the registered subscriptions to the new handler's emitters before dispatching the work that triggered the bind, so an event caused by the very first call on a fresh handler cannot be missed.

Two methods on the returned server complete the lifecycle:

  • detachHandler() — the current object is going away: remove the forwarding listeners from it, keep the registry, drop the handler reference so it can be collected. The next call or subscription invokes the factory again, possibly yielding a different object.
  • ensureHandler() — bind now, without waiting for traffic. Needed when something outside the channel decides the object should exist: a subscribed-but-idle client sends no frames, so events emitted between an out-of-band open and the next inbound frame would otherwise be lost.

Supporting semantics: ON handling is registry-first, so ON/OFF ordering is exact even inside the unbound window, and a subscription survives a factory rejection (attached on the next successful bind). A factory rejection answers each awaited call with the serialized error, code preserved, and is not cached. close() on a factory-backed server answers frames still awaiting a bind with ChannelClosedError rather than dropping them. detachHandler() during an in-flight bind is safe (epoch-guarded); an in-flight streamed response at detach runs to completion against the old handler.

Also included, found while validating downstream: handleOff now removes the registry entry even when the current handler lacks the target emitter (previously the unsubscribe was silently discarded and the subscription resurrected on a later rebind), and the events builtin is imported as node:events with a duck-typed fallback in getNestedEventEmitter — under bundlers that don't externalize the package, 'events' can resolve to the npm shim, which made the instanceof check fail and silently drop every subscription.

Motivation

comapeo-ipc is moving project-instance lifecycle fully behind the server (digidem/comapeo-ipc#88): per-project channels are stable and client references permanent, while the backend closes and re-opens MapeoProject instances freely (leave/re-join, future memory eviction). That requires exactly this: a server whose handler can change behind a durable subscription registry. Implementing it here rather than in comapeo-ipc removes that library's re-implementation of path walking and subscription bookkeeping over wire-frame introspection.

Testing

406 tests passing (65 new assertions across the late-bound suite plus server tests), including: handler swap round-trips, single-flight and epoch semantics, attach-before-dispatch with synchronous emits during the first call, factory rejection and retry, registry-first ON/OFF ordering, GC release of detached handlers (WeakRef under --expose-gc), duck-typed emitters, and a no-reserved-names check. Coverage on server.js is 100% statements/lines/functions. Validated end-to-end downstream by the comapeo-ipc v10 branch, whose lifecycle suite runs entirely on this feature.

No version bump in-branch; the release flow derives the minor bump from the feat: commit on merge.

createServer() now accepts a factory function in place of the handler
object. The channel and its event subscriptions are durable; the handler
is bound lazily (single-flight, identity-aware) and can be released with
detachHandler() and re-bound with ensureHandler() or by the next
incoming call or subscribe. Subscriptions are re-attached before awaited
messages are dispatched so no event from a fresh handler is missed.
Static handler objects behave exactly as before.
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.56%. Comparing base (33af691) to head (fec43e0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #55      +/-   ##
==========================================
+ Coverage   99.33%   99.56%   +0.22%     
==========================================
  Files          12       12              
  Lines        1204     1381     +177     
==========================================
+ Hits         1196     1375     +179     
+ Misses          8        6       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Review fixes for the late-bound handler feature:

- handleOff now removes the registry entry before the emitter lookup, so
  an unsubscribe sticks even when the current handler lacks the emitter
  (previously a later rebind resurrected the subscription).
- handleOn is registry-first: the forwarding listener captures nothing
  from the handler, so it is registered immediately and attached by the
  bind's registry walk. This makes ON/OFF ordering exact while a bind is
  in flight, and subscription intent now survives a factory rejection
  (retained and attached on the next successful bind) instead of being
  dropped with a warning.
- A request still awaiting a bind when the server closes is answered
  with an RPC_CHANNEL_CLOSED error response (or the factory's own error
  if the bind rejects) instead of being silently dropped and left to the
  client timeout.
- Removed the unreachable identity/detach-old guard in bindHandler
  (binds only ever start while unbound) and corrected the README claim
  it implied.
Import from 'node:events' everywhere so bundlers that do not externalize
symlinked packages cannot remap the import to the npm events shim
(hoisted via readable-stream), which made instanceof fail against
handlers built with the builtin EventEmitter and silently dropped every
subscription. Also fall back to duck-typing (on/removeListener/emit) in
getNestedEventEmitter so a handler built against a different EventEmitter
copy (dual node_modules trees) still works; objects failing both checks
still throw.

@RangerMauve RangerMauve left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing major but got some nits

*/

/** @param {number} ms */
function delay(ms) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use p-delay or extract the util?

'Old handler is garbage collected after detach',
)
} else {
t.pass('global.gc not available (run with --expose-gc for the GC check)')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should set this in package.json then? Maybe skip the whole test if unset?

Comment thread test/server.test.js
t.end()
})

function delay(ms) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate

Comment thread server.js
function handleRequest(request) {
const { msgId, method, args } = request
if (!boundHandler) {
const resultPromise = awaitBind().then(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract to own function to keep this one clean.

Comment thread server.js

if (!boundHandler) {
awaitBind().catch((err) => {
log.warn(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be handling this somewhere?

Comment thread server.js
.then(/** @type {HandlerFactory} */ (createHandler))
.then(
(nextHandler) => {
bindPromise = null

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we only null if its our bind promise?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants